Repository navigation
Reject BatchNode serialization until runtime state can be restored - #3071
adrian-andrzjewski wants to merge 1 commit into
Conversation
|
@riccardobl Could you review this fix for #2600? It makes BatchNode and SimpleBatchNode serialization fail clearly because generated batch geometry cannot be distinguished from source geometry after loading. Regression coverage is included for both node types. |
jaime-jmebot
left a comment
There was a problem hiding this comment.
Hi Adrian, thanks for tackling this and for tracing the failure back to the missing runtime batch bookkeeping. Rejecting serialization at the BatchNode boundary is a clear and conservative way to prevent both batched and unbatched instances from being restored into an ambiguous state, and the class documentation makes the limitation visible to users of SimpleBatchNode as well.
I reviewed the exporter and importer overrides alongside the regression tests. Since SimpleBatchNode inherits these methods, the same fail-fast behavior covers both node types, and the tests exercise export after batching as well as the read path. The explicit exception also gives callers a much clearer signal than allowing a silently corrupted scene to load. The change looks focused and consistent with the stated limitation; nice work!
Fixes #2600.
Summary
BatchNodeorSimpleBatchNodeis serialized or deserialized.Root cause
Batch geometries are stored among the node's children, but the runtime batch list and geometry lookup bookkeeping are not serialized. After loading, a saved generated mesh can be mistaken for source geometry when
batch()runs again, producing duplicated batch output. These runtime-generated nodes therefore cannot be restored reliably.Serialization is rejected for unbatched instances too, because a deserializer cannot reliably distinguish logical source geometry from previously generated batch geometry.
Validation
:jme3-core:test --tests com.jme3.scene.BatchNodeSerializationTest— passed.:jme3-core:test— passed.:jme3-core:check— passed; reported Checkstyle warnings are in unrelated existing files.